feat: implement workspace-specific index settings storage #6407
Conversation
- Add workspace storage methods to ContextProxy - Update CodeIndexConfigManager to use workspace storage instead of global - Add migration logic to move existing global settings to workspace - Update webview message handler to save settings per workspace - Add comprehensive tests for workspace-specific storage functionality Fixes #6406
dosubot
bot
added
size:L
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR successfully implements workspace-specific storage for index settings, addressing issue #6406. The implementation includes proper migration from global to workspace settings and comprehensive test coverage.
Critical Issues (Must Fix):
- Missing error handling in ContextProxy.updateWorkspaceState() - The method at lines 143-145 in src/core/config/ContextProxy.ts doesn't handle potential failures. Could this fail silently if workspace state update fails?
Important Suggestions (Should Consider):
-
Inconsistent migration behavior - In src/services/code-index/config-manager.ts lines 47-53, the migration from global to workspace happens every time workspace config is missing. Should this be a one-time operation instead?
-
Potential race condition - In src/core/webview/webviewMessageHandler.ts lines 1993-2012, the workspace state read and write operations aren't atomic, which could lead to data loss if multiple saves happen concurrently.
-
Test coverage gap - The new workspace storage methods in ContextProxy.ts (lines 136-145) lack direct unit tests, only integration tests exist.
Minor Improvements (Nice to Have):
-
Code duplication - The default configuration object in src/services/code-index/config-manager.ts lines 56-65 is duplicated and could be extracted to a constant.
-
Missing JSDoc documentation - The new workspace methods in ContextProxy.ts would benefit from JSDoc comments for better API discoverability.
Positive Notes:
- Excellent test coverage for the workspace-specific functionality
- Clean migration strategy from global to workspace settings
- Proper handling of fallback scenarios
- Implementation correctly addresses the user's need for different configurations per workspace
Overall, this is a solid implementation that solves the core issue effectively. The critical error handling issue should be addressed, but the functionality works as intended.
hannesrudolph
added
the
Issue/PR - Triage
|
Closing, see #6406 (comment) |
Implements workspace-specific storage for index settings to fix issue #6406.
Key changes:
This allows different workspaces to have different vector database configurations, which is especially useful when switching between local development and dev containers.
Fixes #6406
Important
Implements workspace-specific storage for index settings, enabling distinct configurations per workspace and automatic migration from global settings.
ContextProxyandCodeIndexConfigManager.webviewMessageHandlerto handle per-workspace saving of settings.config-manager.spec.tsto verify workspace-specific storage, migration, and preference over global settings.This description was created by
for 6d414c3. You can customize this summary. It will automatically update as commits are pushed.